-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add jsdoc
bundle package
#10
Conversation
(cherry picked from commit cb88933) Co-authored-by: Miguel Perez Pellicer <[email protected]>
as npm fails to resolve shelljs dependency.
from `-tracking-jsdoc`. It has nothing to do with Tracking, it's just our code convention.
as it is not implemented.
from the `-jsdoc` readme. The plugin should be already installed given the budnle is.
* Internal dependencies | ||
*/ | ||
import { shell } from '../utils.js' | ||
process.env.PATH += ( path.delimiter + path.join( process.cwd(), 'node_modules', '.bin') ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is repeated over all the bin files, so I guess could be reuse it in utils.js
kinda file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In all 2 ;) We cannot.
as npm fails to resolve shelljs dependency in a parent folder without its own package.json
9914196
Plus it's single line
/**
* Internal dependencies
*/
import { shell } from '../../utils.js'
vs.
import shell from 'shelljs';
import path from 'path';
process.env.PATH += ( path.delimiter + path.join( process.cwd(), 'node_modules', '.bin') );
So not a big difference in LOC, and a bit more explicit, that it's just the original shelljs
When I ran the installation I got this error:
|
instead of relative FS path, to make the `jsdoc` package installable elsewhere. Related to #5 Address #10 (comment)
Thanks, it was a leftover from my local testing with a relative filesystem reference. Fixed in 8b88bfd |
@puntope would you mind another look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ LGTM
I was able to install the plugin and generate the tracking docs
Thanks ❤️
Changes proposed in this Pull Request:
Addresses some of the concerns from #6 (review)
Bundles multiple commonly used
jsdoc
plugins, so the extensions repo would need only this one dependency.Exports a binary, that wraps commonly used
jsdoc
CLI arguments, for easier use.Screenshots:
Detailed test instructions:
See example usage at woocommerce/google-listings-and-ads@develop...tweak/bundled-jsdoc
And the installation & usage instructions at https://github.com/woocommerce/grow/blob/add/jsdoc-bundle/packages/js/jsdoc/README.md
npm i --save-dev https://gitpkg.now.sh/woocommerce/grow/packages/js/jsdoc?add/jsdoc-bundle
TRACKING.md
file.jsdocrc.json
npx woocommerce-grow-jsdoc
Additional details:
Changelog entry